Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mutate relevant compilation variables into config #1256

Merged
merged 18 commits into from
Aug 20, 2022

Conversation

BadWolf42
Copy link
Contributor

@BadWolf42 BadWolf42 commented Aug 14, 2022

Hello Team,

Description:

As per issue #1240, this PR removes major non-hardware dependent compilation variables from ZgatewayBT.ino.

To achieve this, the following changes have been made:

  • Move variable minRssi in config_BT.h,
  • Create a struct (BTConfig_s) for BT configuration data,
  • Create a default config for BTConfig_s (BTConfig_default) based on defines,
  • Create a global variable (BTConfig) to hold live BT configuration data and initialize it,
  • Move global variables bleConnect, BLEscanBeforeConnect, BLEinterval, PublishOnlySensors, hassPresence, minRssi into struct BTConfig_s,
  • Add presenceTopic into BTConfig_s,
  • Move MQTTDecodeTopic as extDecoderTopic into BTConfig_s,
  • Move UseExtDecoder, BLE_FILTER_CONNECTABLE, pubKnownBLEServiceData, pubUnknownBLEServiceData, pubBLEManufacturerData, pubUnknownBLEManufacturerData, pubBLEServiceUUID, useBeaconUuidForTopic, `useBeaconUuidForPresence as runtime var into BTConfig_s

Also compilation variable AttemptBLECOnnect has been renamed AttemptBLEConnect, which could be A BREAKING CHANGE.
And compilation variable ServicedataMinLength has been removed (unused).

The next steps are:

  1. Provide the ability to modify those configurations with mqtt payloads on MQTTtoBT topic,
  2. Review White/Black-lists to handle "wildcard" mac adresses/uuid (point raised in issues Ibeacons with random MAC addresses #1139, iBeacon: Flag to use UUID in topic or presence (and fixes) #1226 and PR Basic whitelisting/blacklisting support for ZgatewayRF #919 for ZgatewayRF) and add a parameter to disable black/white-listing (point raised in issue BLE Whitelist. #331 and PR Add support to disable/enable white list control #615),
  3. Persist configuration change in SPIFFS instead of having to retain the configuration in the Broker.

Tell me if this is going the right way for you.

Bad

Checklist:

  • The pull request is done against the latest development branch
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • I accept the DCO.

Move var definitions next to compilation var definitions
Replace #ifdef MQTTDecodeTopic by #if UseExtDecoder
Create a default config for BTConfig_s (BTConfig_default) based on defines
Create a global variable (BTConfig) to hold live BT configuration data and initialize it
Move bleConnect to BTConfig
Always declare AttemptBLEConnect & BLE_FILTER_CONNECTABLE
Mutate pubUnknownBLEServiceData as runtime var in BTConfig
Mutate pubBLEManufacturerData as runtime var in BTConfig
Mutate pubUnknownBLEManufacturerData as runtime var in BTConfig
Mutate pubBLEServiceUUID as runtime var in BTConfig
Mutate useBeaconUuidForPresence as runtime var in BTConfig
@BadWolf42
Copy link
Contributor Author

Lint issues are regarding struct BTConfig_s and BTConfig_default formating/indentation (for readability).
A can conform to lint suggestion but I also wait for your advice on this point.
Thanks

@1technophile
Copy link
Owner

Hello @BadWolf42 , I formatted it.
If you want to install the format, you have to add "clang-format" extension to PIO.
Right click into the code-> Format document

@BadWolf42
Copy link
Contributor Author

Ok, got it, thanks.
Let me know what you think about this PR and the others to come.

@1technophile
Copy link
Owner

Give me a few days to test it and I will review it

@1technophile
Copy link
Owner

Hello @BadWolf42 , LGTM, thanks

@1technophile 1technophile merged commit 9120d06 into 1technophile:development Aug 20, 2022
@BadWolf42
Copy link
Contributor Author

Thank you, I'll continue asap on the next step

@BadWolf42 BadWolf42 deleted the mutate-def-to-config branch August 20, 2022 19:40
@BadWolf42 BadWolf42 mentioned this pull request Aug 26, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants